client: get site from go:embed #1710
Conversation
|
This looks pretty slick. How do we get the tests to pass? |
vctt94
force-pushed
the
get-site-from-goembed
branch
2 times, most recently
from
edd8952
to
f3fa5fe
Compare
|
tests passing now. With this PR we will need to run: |
vctt94
force-pushed
the
get-site-from-goembed
branch
from
f3fa5fe
to
db41949
Compare
|
Just to reiterate the discussion in matrix, we need a way to allow CI to function without checking in the build output (dist). Perhaps just some placeholder file in the dist folder like The lack of the expected files in either (a) embed.FS OR (b) the actual file system, should result in a runtime error or just a 404 on the webserver as it would now, not a compile-time error. |
|
The other obvious solution is to merge the CI tasks so that the JS build and Go build are in the same runner. That would be a bummer though because the build matrix is unnecessarily large in that case. |
There was a problem hiding this comment.
A few changes, please.
- Don't check in the dist files.
- Let's put some empty dummy files (entry.js and style.css) in client/webserver/ for the /dist files. That'll get us past the tests.
//go:embed entry.js
jsFile embed.FS
//go:embed style.css
cssFile embed.FS- We can recognize the empty files in
New, and use the old search. - In the future, the packaging script can hot swap the compiled filed during the build. No need to do that in the PR.
Or maybe @chappjc's is right about just using an empty directory
What I meant was this: The Go binary will build now. It would just 404 in the file server handler, but it can fall back to disk instead. For releases, we can check in the final |
|
This is very close. I'm going to suggest a diff in a few. |
|
Here's what I think we should change: d86dd40 When we start distributing Go binaries with these files embedded, when users upgrade they are likely to overwrite the old dexc(.exe) and leave the old site files. As such, by default dexc needs to only use the embedded files, not first try disk nor fallback to disk. The After building, we can verify the embedded files with: Default: With With With Subsequent work should look at removing the |
chappjc
force-pushed
the
get-site-from-goembed
branch
from
db41949
to
90cb769
Compare
| echo "Files embedded in the Go webserver package:" | ||
| go list -f '{{ .EmbedFiles }}' decred.org/dcrdex/client/webserver |
There was a problem hiding this comment.
Here's how we'll verify that the binaries have the intended site files embedded.
There was a problem hiding this comment.
I think I'm mistaken about the go list command. e.g. I make the build with the dist dir and it embeds, then I can do the go list command and it shows the files. Then I rm -r dist and do the list command again:
$ go list -f '{{ .EmbedFiles }}' decred.org/dcrdex/client/webserver
../webserver.go:81:13: pattern site/dist: no matching files found
So clearly that go list is not just checking the compiled webserver package in the go package cache or whatever, but something based on the current files.
In this pkg script it probably is doing what we expect, but in general it's not.
| # Generate the localized_html and build the webpack bundle prior to building the | ||
| # webserver package, which embeds the files. | ||
| pushd client/webserver/site | ||
| go generate # should be a no-op |
There was a problem hiding this comment.
Just wondering, what's the reason for this?
There was a problem hiding this comment.
Make sure the templates in localized_html correspond to the sources. CI makes us check them in presently so it's likely a no-op in this script if it's being run on an unmodified git revision.
There was a problem hiding this comment.
This is what I see after 1) switch to this branch, 2) go build -tags lgpl, 3) ./dex --simnet --rpc --log=debug
No asset logos, but I can see e.g. the dcrdex logo (from a different directory). If I
go list -f '{{ .EmbedFiles }}' decred.org/dcrdex/client/webserverI can see all of the image files.
On the front side, I'm seeing status 200 on the images requests, but the files appear to be empty.
I'm running with the cache disabled.
|
I managed to repro the "empty" files. For me it randomly made the big logo (logo_wide_dark_v1.svg) blank.
Point the browser directly at it:
The reason seems to be a MIME issue:
(png instead of svg). I downloaded the file and the contents were correct. Not clear why it only sometimes happens, but it's gotta be related to content type so we know what do debug I think. |
Co-authored-by: Jonathan Chappelow <chappjc@gmail.com>
chappjc
force-pushed
the
get-site-from-goembed
branch
from
90cb769
to
2ab762e
Compare
|
Np. Sorry for rushing it as a late addition to 0.5 |
This PR closes #1664
For testing can run
go installin dexc directory and make sure that the foldersiteisn't in the go executable's directory or the working directory.